Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Too many warnings make it hard for users to see things they should be actually warned about. In this case, users use field-lists to visually group questions and that grouping really doesn't need a label, so we don't need to warn users.
And while I was looking at the warnings, I wanted to improve them by specifying that the warning was about a group or a repeat.
I also removed the warning for username and start-geopoint. The later, I'm not sure ever threw a warning.
Why is this the best possible solution? Were any other approaches considered?
Originally, I wanted to warn on every group and every repeat, but decided to dial it back because...
What are the regression risks?
There could be some downstream tools that rely on group labels, but since has always been a warning and not an error, it seems likely that those tools can handle missing group labels.
There doesn't seem to be a fixed list of acceptable ways to say "begin group". There is a regex in https://github.com/XLSForm/pyxform/blob/master/pyxform/xls2json.py#L570 that I built my tests around.
Finally, there isn't a lot of test coverage on groups, so that didn't make me feel great.
Does this change require updates to documentation? If so, please file an issue here and include the link below.
No, infact https://xlsform.org/en/#multiple-webpage-forms shows an example of a field-list without a label.
Before submitting this PR, please make sure you have:
tests_v1
nosetests
and verified all tests passblack pyxform
to format code